Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text with onPress or onLongPress handler is not accessible with TalkBack #34284

Conversation

fabOnReact
Copy link
Contributor

@fabOnReact fabOnReact commented Jul 27, 2022

Summary

Finally, the last catch relates to why these views are considered focusable. We've been working with the assumption that they are only focusable because accessible="true", but this is not the only property that can make a view focusable on Android. Android also makes all elements with onClick listeners or onLongPress listeners focusable

Adding onPress handler to a Text Component does not call setClickable(true) (test case) #30851 (comment)

Pressable, TouchableOpacity, Switch, TextInput, and TouchableNativeFeedback are focusable/accessible by default without an onPress handler or accessible prop.

<TouchableOpacity />

The TouchableOpacity is accessible

<TouchableOpacity accessible={false} />

The TouchableOpacity is not accessible

<TouchableOpacity accessible={false} onPress={() => console.log('pressed')} />

The TouchableOpacity is accessible.

focusable={
this.props.focusable !== false && this.props.onPress !== undefined
}

This and other PRs fixes #30851

Changelog

[Android] [Fixed] - Text with onPress or onLongPress handler is not accessible with TalkBack

Test Plan

main branch #30

pr branch

2022-07-27.17-00-33.mp4

>Finally, the last catch relates to why these views are considered focusable. We've been working with the assumption that they are only focusable because accessible="true", but this is not the only property that can make a view focusable on Android. Android also makes all elements with onClick listeners or onLongPress listeners focusable, as well as a bunch of other less-clear edge cases. If our layout looked like this:

adding onPress handler to a Text Component does not call setClickable(true) ([test case](facebook#30851 (comment)))

facebook#30851 (comment)

verify if the accessible prop does not work on other components
      * Pressable, TouchableOpacity, Switch, TextInput, and TouchableNativeFeedback are focusable/accessible by default without an onPress handler
      * `<TouchableOpacity>` is accessible, `<TouchableOpacity accessible={false}>` is not accessible, `<TouchableOpacity accessible={false} onPress={() => console.log('pressed')}>` is accessible

https://github.com/facebook/react-native/blob/a70354df12ef71aec08583cca4f1fed5fb77d874/Libraries/Components/Touchable/TouchableOpacity.js#L249-L251
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 27, 2022
@fabOnReact
Copy link
Contributor Author

fabOnReact commented Jul 27, 2022

  • build on iOS and Android after deleting .gradle cache and rebasing
  • test iOS iCloud issues - Xcode crashes in project, need to troubleshoot again tomorrow
  • test onLongPress
test onLongPress

2022-08-04.18-56-34.mp4

@analysis-bot
Copy link

analysis-bot commented Jul 27, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,618,089 -24
android hermes armeabi-v7a 7,032,343 -21
android hermes x86 7,918,142 -24
android hermes x86_64 7,891,761 -21
android jsc arm64-v8a 9,495,493 +29
android jsc armeabi-v7a 8,273,119 +31
android jsc x86 9,433,278 +29
android jsc x86_64 10,026,271 +37

Base commit: b444f0e
Branch: main

@analysis-bot
Copy link

analysis-bot commented Jul 27, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: b444f0e
Branch: main

@fabOnReact fabOnReact marked this pull request as ready for review August 4, 2022 11:23
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Aug 4, 2022
@blavalla
Copy link
Contributor

This looks good to me, but I just wanted to confirm that you were able to test this change on iOS as well.

@fabOnReact fabOnReact marked this pull request as draft August 30, 2022 04:40
@fabOnReact fabOnReact force-pushed the text-onPress-does-not-trigger-accessible branch from 39c269a to 93d3bc0 Compare August 30, 2022 05:18
@fabOnReact
Copy link
Contributor Author

fabOnReact commented Aug 30, 2022

sorry, @blavalla. I had again this issue with iCloud, and I postponed testing iOS.

Test before commit 4149b68
Test after commit 4149b68

As included in the documentation pr

Android
In the above example, the TalkBack screenreader focuses on the parent view with the 'accessible' property and uses the unfocusable child Text with the text "text two" as a label. The focus will then move to read the second focusable child Text with the text "text one".
<View
  accessible={true}
  accessibilityLabel="a couple of text views">
  <Text onPress={() => console.log('text one pressed')}>
    text One
  </Text>
  <Text onPress={() => console.log('text two pressed')}>
    text Two
  </Text>
</View>
iOS
VoiceOver can't get accessibility focus separately on 'text one' and 'text two' in the above example. Instead, it focuses on the parent view with 'accessible' property.

@fabOnReact fabOnReact marked this pull request as ready for review August 30, 2022 09:45
@facebook-github-bot
Copy link
Contributor

@blavalla has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @fabriziobertoglio1987 in f3847ee.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Sep 20, 2022
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…ack (facebook#34284)

Summary:
>Finally, the last catch relates to why these views are considered focusable. We've been working with the assumption that they are only focusable because accessible="true", but this is not the only property that can make a view focusable on Android. Android also makes all elements with onClick listeners or onLongPress listeners focusable

Adding onPress handler to a Text Component does not call setClickable(true) ([test case](facebook#30851 (comment))) facebook#30851 (comment)

Pressable, TouchableOpacity, Switch, TextInput, and TouchableNativeFeedback are focusable/accessible by default without an onPress handler or accessible prop.

```jsx
<TouchableOpacity />
```
The TouchableOpacity is accessible

```jsx
<TouchableOpacity accessible={false} />
```
The TouchableOpacity is not accessible

```jsx
<TouchableOpacity accessible={false} onPress={() => console.log('pressed')} />
```

The TouchableOpacity is accessible.

https://github.com/facebook/react-native/blob/a70354df12ef71aec08583cca4f1fed5fb77d874/Libraries/Components/Touchable/TouchableOpacity.js#L249-L251

This and other PRs fixes facebook#30851

## Changelog

[Android] [Fixed] - Text with onPress or onLongPress handler is not accessible with TalkBack

Pull Request resolved: facebook#34284

Test Plan:
main branch facebook#30

<details><summary>pr branch</summary>
<p>

<video src="https://user-images.githubusercontent.com/24992535/181207388-bbf8379b-71b8-44e9-b4b2-b5c44e9ac14d.mp4" width="1000" />
</p>
</details>

Reviewed By: cipolleschi

Differential Revision: D39179107

Pulled By: blavalla

fbshipit-source-id: 3301fb2b799f233660e3e08f6a87dad294ddbcd8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android: "accessible" prop issues.
5 participants